Skip to content

Pin monarch lib to a commit SHA and document the fork landscape#9

Merged
jamiew merged 1 commit into
mainfrom
bump-monarch-lib-pin
May 29, 2026
Merged

Pin monarch lib to a commit SHA and document the fork landscape#9
jamiew merged 1 commit into
mainfrom
bump-monarch-lib-pin

Conversation

@jamiew

@jamiew jamiew commented May 29, 2026

Copy link
Copy Markdown
Owner

bumps our monarch client pin and writes down what i learned analyzing the upstream fork landscape, since this library is a fork-of-a-fork and that context kept getting lost.

  • moved the monarchmoneycommunity pin from the moving rev=main to the fork's dev HEAD commit (befdadc) — reproducible builds, plus it picks up the new reviewed param on update_transaction
  • added an "Upstream Library & Fork Landscape" section to CLAUDE.md: the parent (hammem, effectively abandoned), the community fork we actually use, unused methods we already ship but don't wrap as tools, and a checklist for re-running this analysis
  • flagged keithah/monarchmoney-enhanced as a followup — adds a rules engine, caching, session validation, goals, bills, insights, several of which map onto existing TODOs — but it's not a drop-in and needs live testing first
  • fixed the stale dep line that still said monarchmoney ≥0.1.15

to test:

  • uv sync && uv run python scripts/ci.py → all green
  • confirm update_transaction now accepts a reviewed kwarg

- bump monarchmoneycommunity pin from moving `rev=main` to the fork's `dev`
  HEAD SHA (befdadc) — gains the `reviewed` param on update_transaction and
  makes builds reproducible
- add an "Upstream Library & Fork Landscape" section to CLAUDE.md covering the
  parent/community/enhanced forks, our pin, unused capabilities, and a
  how-to-keep-current checklist
- fix the stale dependency line that still referenced `monarchmoney ≥0.1.15`
@claude

claude Bot commented May 29, 2026

Copy link
Copy Markdown

Claude finished @jamiew's task in 1m 0s —— View job


Overall: Approve. Clean dependency hygiene, zero functional risk, and genuinely useful documentation that prevents this context from getting lost again.

View full review

pyproject.toml — pin change

The move from rev = "main" to a full 40-character SHA is exactly right. rev = "main" is a moving target: uv sync on a fresh clone could silently resolve to a different commit than what CI saw, depending on when it runs. The full SHA makes builds fully reproducible.

The inline comment is helpful:

# Pinned to a specific commit (not a moving branch) for reproducible builds.
# This is the fork's `dev` HEAD as of 2026-05-28 — see "Upstream library" in CLAUDE.md.
monarchmoneycommunity = { git = "...", rev = "befdadc7163f90053be5820346f3095b78c7a857" }

One minor note: the cross-reference to CLAUDE.md is good, but "Upstream library" doesn't quite match the section header "Upstream Library & Fork Landscape" — close enough that it's not a blocker, but a precise anchor would make the grep easier (CLAUDE.md#upstream-library--fork-landscape or just the exact heading text).

uv.lock — lock file update

The lock file correctly reflects the SHA change. Note that when pinning to a specific commit, uv sets both rev= and the resolved #sha to the same value (vs. rev=main#resolved-sha when tracking a branch) — this is expected and correct behavior.

-source = { git = "...?rev=main#4a26e220e03388c06603fffa3f22bed5714fe463" }
+source = { git = "...?rev=befdadc7...#befdadc7..." }

Version stays at 1.3.2 — the library's pyproject.toml at this commit still declares that version, which is fine.

CLAUDE.md — fork landscape documentation

This is the most valuable part of the PR and it's well done.

Strengths:

  • The table format clearly ranks repos by relevance to this project
  • Calling out the specific broken PRs (#184, #191) in the parent repo is useful — future-you will want exactly those numbers when checking if hammem has recovered
  • "Not a drop-in" caveat on keithah/monarchmoney-enhanced is the right framing; it prevents someone from doing a one-line dep swap without testing
  • The "Unused capabilities" list is a genuinely useful shortcut — zero new dependencies, just wrapper work
  • The gh api commands in "How to keep this current" are concrete enough to actually run

Minor suggestions (none blocking):

  1. No test for reviewed kwarg. The PR body says to "confirm update_transaction now accepts a reviewed kwarg" but there's no automated coverage for it. Given that this is the main functional gain from the pin bump, a small test (even just checking the kwarg passes through without error) would lock in the behavior. Fix this →

  2. refresh_accounts vs request_accounts_refresh_and_wait. The "unused capabilities" list includes request_accounts_refresh_and_wait, but the tools list in the same file already shows refresh_accounts as an implemented tool. These are likely different methods (one blocks until complete, one fires-and-forgets), but the distinction is worth a one-word clarification ("fire-and-forget version already wrapped as refresh_accounts; _and_wait variant not yet").

  3. Last checked date. The section opens with "Last checked: 2026-05-28" — this is yesterday, correct. When bumping again, the checklist in step 5 already covers updating this, so the process is self-maintaining.

Security

Pinning to a commit SHA is strictly better from a supply chain perspective than a branch name. No regressions here.

Summary

Area Verdict
Dependency pin ✅ Correct, reproducible
Lock file ✅ Consistent with pin
Documentation ✅ High-quality, actionable
Test coverage for reviewed kwarg ⚠️ Missing (minor)

@jamiew jamiew merged commit 57a212d into main May 29, 2026
6 checks passed
@jamiew jamiew deleted the bump-monarch-lib-pin branch May 29, 2026 02:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant